Skip to content

Improve oracle support in reconcile #2081

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 28 commits into from
Oct 29, 2025
Merged

Improve oracle support in reconcile #2081

merged 28 commits into from
Oct 29, 2025

Conversation

@m-abulazm
Copy link
Contributor

@m-abulazm m-abulazm commented Oct 6, 2025 •
edited
Loading

Changes

What does this PR do?

  • Fix Oracle JDBC URL: move credentials out of URL into options, thin syntax corrected. Pass user and password through options
  • Hashing/expr pipeline: replace bad RAWTOHEX(...), 2 with UTL_I18N.STRING_TO_RAW(...,'AL32UTF8'), 4 (SHA-256).
  • Small fix for oracle and snowflake schema compare.
  • default transformations: tweak datatype parsing so oracle works correctly.
  • Log levels: demote noisy warnings to debug/info.
  • Infra: add Oracle/Snowflake jars in setup script.
  • Tests: extend integration scaffolding for Oracle/Snowflake, add e2e test.

Linked issues

Resolves #2057

Functionality

  • added relevant user documentation

Tests

  • manually tested
  • added unit tests
  • added integration tests

@github-actions
Copy link

github-actions bot commented Oct 6, 2025 •
edited
Loading

✅ 46/46 passed, 6 flaky, 3m25s total

Flaky tests:

  • 🤪 test_validate_non_empty_tables (37ms)
  • 🤪 test_transpiles_informatica_to_sparksql (15.713s)
  • 🤪 test_transpile_teradata_sql_non_interactive[True] (15.868s)
  • 🤪 test_transpile_teradata_sql (20.091s)
  • 🤪 test_transpiles_informatica_to_sparksql_non_interactive[True] (8.289s)
  • 🤪 test_transpile_teradata_sql_non_interactive[False] (11.611s)

Running from acceptance #2800

@m-abulazm m-abulazm self-assigned this Oct 6, 2025
@gitguardian
Copy link

gitguardian bot commented Oct 9, 2025 •
edited
Loading

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.

🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@m-abulazm m-abulazm marked this pull request as ready for review October 13, 2025 10:23
@m-abulazm m-abulazm requested a review from a team as a code owner October 13, 2025 10:23
@codecov
Copy link

codecov bot commented Oct 22, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 62.96296% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.92%. Comparing base (7be6421) to head (eaa68ad).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...bricks/labs/lakebridge/reconcile/schema_compare.py 0.00% 5 Missing ⚠️
...labs/lakebridge/reconcile/trigger_recon_service.py 0.00% 3 Missing ⚠️
...rc/databricks/labs/lakebridge/reconcile/compare.py 0.00% 1 Missing ⚠️
...abricks/labs/lakebridge/reconcile/recon_capture.py 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2081      +/-   ##
==========================================
+ Coverage   64.87%   64.92%   +0.04%     
==========================================
  Files          96       96              
  Lines        7895     7908      +13     
  Branches      821      822       +1     
==========================================
+ Hits         5122     5134      +12     
- Misses       2593     2595       +2     
+ Partials      180      179       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

sundarshankar89
sundarshankar89 reviewed Oct 28, 2025
View reviewed changes
Copy link
Collaborator

@sundarshankar89 sundarshankar89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM Small nit around NVARCHAR handling.

sundarshankar89
sundarshankar89 approved these changes Oct 28, 2025
View reviewed changes
Copy link
Collaborator

@sundarshankar89 sundarshankar89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@m-abulazm m-abulazm enabled auto-merge October 29, 2025 09:08
@m-abulazm m-abulazm added this pull request to the merge queue Oct 29, 2025
Merged via the queue into main with commit 3002743 Oct 29, 2025
9 of 10 checks passed
@m-abulazm m-abulazm deleted the fix/recon/oracle branch October 29, 2025 09:45
goodwillpunning pushed a commit that referenced this pull request Nov 5, 2025
@m-abulazm @goodwillpunning
Improve oracle support in reconcile (#2081)
befb4a5 
<!-- REMOVE IRRELEVANT COMMENTS BEFORE CREATING A PULL REQUEST -->
## Changes
<!-- Summary of your changes that are easy to understand. Add
screenshots when necessary, they're helpful to illustrate the before and
after state -->
### What does this PR do?
* Fix Oracle JDBC URL: move credentials out of URL into options, thin
syntax corrected. Pass user and password through options
* Hashing/expr pipeline: replace bad `RAWTOHEX(...), 2` with
`UTL_I18N.STRING_TO_RAW(...,'AL32UTF8'), 4` (SHA-256).
* Small fix for oracle and snowflake schema compare.
* default transformations: tweak datatype parsing so oracle works
correctly.
* Log levels: demote noisy warnings to debug/info.
* Infra: add Oracle/Snowflake jars in setup script. 
* Tests: extend integration scaffolding for Oracle/Snowflake, add e2e
test.


### Linked issues
<!-- DOC: Link issue with a keyword: close, closes, closed, fix, fixes,
fixed, resolve, resolves, resolved. See
https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword
-->

Resolves #2057 

### Functionality

- [ ] added relevant user documentation

### Tests
<!-- How is this tested? Please see the checklist below and also
describe any other relevant tests -->

- [X] manually tested
- [ ] added unit tests
- [X] added integration tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gueniai gueniai gueniai approved these changes

@sundarshankar89 sundarshankar89 sundarshankar89 approved these changes

@bishwajit-db bishwajit-db Awaiting requested review from bishwajit-db

@ganeshdogiparthi-db ganeshdogiparthi-db Awaiting requested review from ganeshdogiparthi-db

Assignees

@m-abulazm m-abulazm

Labels

bug Something isn't working feat/recon making sure that remorphed query produces the same results as original

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG]: Reconcile, false mismatches , incomplete data in reconcile.details

4 participants

@m-abulazm @gueniai @sundarshankar89